Skip to content

[CHROE] main 병합 및 배포#163

Merged
chltjsdl0119 merged 10 commits intomainfrom
develop
Feb 18, 2026
Merged

[CHROE] main 병합 및 배포#163
chltjsdl0119 merged 10 commits intomainfrom
develop

Conversation

@chltjsdl0119
Copy link
Collaborator

No description provided.

@chltjsdl0119 chltjsdl0119 self-assigned this Feb 18, 2026
Copilot AI review requested due to automatic review settings February 18, 2026 18:04
@chltjsdl0119 chltjsdl0119 merged commit a950409 into main Feb 18, 2026
7 checks passed
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates request/exception/logging behavior as part of a “main merge & deploy” effort, including propagating request IDs, refactoring the logging aspect, adjusting a friends list default sort, and updating related tests.

Changes:

  • Refactor MdcLoggingFilter to OncePerRequestFilter, read/write X-Request-ID, and clear MDC after request completion.
  • Simplify LogAspect logging format and execution timing logic.
  • Update exception advice logging format and adjust controller default pageable sort.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/test/java/com/loopon/global/log/MdcLoggingFilterTest.java Updates tests for new X-Request-ID + MDC cleanup behavior.
src/test/java/com/loopon/global/log/LogAspectTest.java Simplifies aspect test but significantly reduces verification scope.
src/main/java/com/loopon/user/presentation/FriendController.java Changes default sort to updatedAt DESC.
src/main/java/com/loopon/global/log/MdcLoggingFilter.java Implements request-id propagation via header + MDC in a once-per-request filter.
src/main/java/com/loopon/global/log/LogAspect.java Refactors pointcuts and logging output format/timing.
src/main/java/com/loopon/global/exception/GlobalExceptionAdvice.java Adds request context to logs; removes a previously present exception handler.
Comments suppressed due to low confidence (1)

src/main/java/com/loopon/global/exception/GlobalExceptionAdvice.java:109

  • Must fix: AuthorizationException handling was removed from GlobalExceptionAdvice. This exception is thrown in auth/JWT flows with a specific ErrorCode and status; without a dedicated handler it will fall into the generic Exception handler and return 500 instead of the intended error status. Restore an @ExceptionHandler(AuthorizationException.class) that returns ex.getErrorCode().getStatus() and CommonResponse.onFailure(ex.getErrorCode()) (and include request info in the log if desired).
    @ExceptionHandler(Exception.class)
    public ResponseEntity<CommonResponse<Void>> handleException(Exception ex, HttpServletRequest request) {
        log.error("[ERR] {} {} | Exception: ", request.getMethod(), request.getRequestURI(), ex);

        return ResponseEntity
                .status(ErrorCode.INTERNAL_SERVER_ERROR.getStatus())
                .body(CommonResponse.onFailure(ErrorCode.INTERNAL_SERVER_ERROR));
    }


@Pointcut("execution(* com.loopon..*Controller.*(..))")
public void controllerMethods() {
@Pointcut("execution(* com.loopon.*Controller.*(..))")
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must fix: controller() pointcut is now execution(* com.loopon.*Controller.*(..)), which will not match controllers under subpackages like com.loopon.user.presentation.FriendController (and other ...presentation.*Controller). This effectively disables controller logging. Use a recursive package pattern (e.g., com.loopon..*Controller.*(..)), or otherwise align the pointcut to the actual controller package structure.

Suggested change
@Pointcut("execution(* com.loopon.*Controller.*(..))")
@Pointcut("execution(* com.loopon..*Controller.*(..))")

Copilot uses AI. Check for mistakes.
Comment on lines +64 to 67
private String formatArgs(Object[] args) {
if (args == null || args.length == 0) return "[]";
return Arrays.toString(args);
}
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must fix: The aspect now logs arguments via Arrays.toString(args) without any masking. This can leak credentials/tokens/passwords in controller/service logs (the previous masking logic was removed). Reintroduce sensitive-data masking (by parameter name and/or by DTO field annotations) or avoid logging full arguments for authentication-related endpoints.

Copilot uses AI. Check for mistakes.
Comment on lines 27 to +45
public Object logExecution(ProceedingJoinPoint joinPoint) throws Throwable {
StopWatch stopWatch = new StopWatch();
MethodInfo methodInfo = null;
long startTime = System.currentTimeMillis();
Object result = null;

try {
methodInfo = extractMethodInfo(joinPoint);

stopWatch.start();
Object result = joinPoint.proceed();
stopWatch.stop();

logSuccess(methodInfo, result, stopWatch);
result = joinPoint.proceed();
return result;

} catch (Throwable e) {
if (stopWatch.isRunning()) {
stopWatch.stop();
}

logFailure(methodInfo, e, stopWatch);
throw e;
} finally {
long totalTime = System.currentTimeMillis() - startTime;
MethodSignature signature = (MethodSignature) joinPoint.getSignature();

log.info("[{}] {}.{} | Args: {} | Ret: {} | {}ms",
getLayer(signature), // 1. 레이어 (API / SVC)
className(signature), // 2. 클래스명
methodName(signature), // 3. 메서드명
formatArgs(joinPoint.getArgs()), // 4. 파라미터 (단순화)
formatResult(result), // 5. 결과값 (길이 제한)
totalTime // 6. 소요시간
);
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must fix: Exception cases are no longer logged as errors. Because logging happens only in finally with log.info(...), a thrown exception will produce an INFO log with Ret: void and no exception details/stacktrace, which is a regression in observability. Capture the thrown exception (e.g., catch (Throwable t)), log at WARN/ERROR with details, then rethrow; keep the finally for timing/MDC cleanup if needed.

Copilot uses AI. Check for mistakes.
Comment on lines +28 to +43
@DisplayName("결과값이 100자를 넘으면 잘라야 한다")
void truncateResult() throws Throwable {
// given
String longString = "A".repeat(200);
given(joinPoint.proceed()).willReturn(longString);

// When
logAspect.logExecution(joinPoint);

// Then
assertThat(output.getOut())
.contains(longString.substring(0, 100) + "...")
.doesNotContain(longString);
}

private void setupJoinPoint(Class<?> targetClass, String methodName, String[] paramNames, Object[] args) {
doReturn(targetClass).when(methodSignature).getDeclaringType();
given(joinPoint.getSignature()).willReturn(signature);
given(signature.getDeclaringType()).willReturn(TestController.class);
given(signature.getName()).willReturn("testMethod");
given(joinPoint.getArgs()).willReturn(new Object[]{});

given(methodSignature.getName()).willReturn(methodName);
given(methodSignature.getParameterNames()).willReturn(paramNames);
// when
Object result = logAspect.logExecution(joinPoint);

given(joinPoint.getSignature()).willReturn(methodSignature);
given(joinPoint.getArgs()).willReturn(args);
// then
assertEquals(longString, result);
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must fix (tests): truncateResult() no longer verifies truncation behavior. The aspect truncates only in the log message, but this test asserts the returned value equals the full 200-character string and does not capture/assert log output, so it won't fail if truncation is broken (and the DisplayName is misleading). Add log output assertions (e.g., via OutputCaptureExtension) or adjust the test/DisplayName to match what is actually being tested.

Copilot generated this review using guidance from repository custom instructions.
Comment on lines +25 to +39
@DisplayName("헤더에 ID가 없으면 새로 생성하고 MDC에 넣어야 한다")
void generateRequestId() throws Exception {
// given
MockHttpServletRequest req = new MockHttpServletRequest();
MockHttpServletResponse res = new MockHttpServletResponse();
FilterChain chain = mock(FilterChain.class);

// when
filter.doFilter(req, res, chain);

// then
String requestId = res.getHeader("X-Request-ID");
assertNotNull(requestId);
verify(chain, times(1)).doFilter(req, res);
assertNull(MDC.get("request_id"));
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussion needed (tests): MdcLoggingFilterTest no longer verifies that the MDC value is present during filterChain.doFilter(...), nor that MDC cleanup still happens when the downstream chain throws. The current assertions only check the response header and that MDC is empty after completion, so regressions in MDC scoping/exception cleanup could slip through. Consider stubbing the chain to assert MDC inside the invocation and adding an exception-path test.

Copilot generated this review using guidance from repository custom instructions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants